Skip to content

runtime: block signal_recv on targets without signal delivery (#5619) - #5620

Open
neomantra wants to merge 1 commit into
tinygo-org:devfrom
neomantra:nm-signal-stub
Open

runtime: block signal_recv on targets without signal delivery (#5619)#5620
neomantra wants to merge 1 commit into
tinygo-org:devfrom
neomantra:nm-signal-stub

Conversation

@neomantra

Copy link
Copy Markdown
Contributor

This was worked through with LLM. The text below is LLM-generated and I have read and reviewed all the code.

But I did finally get the BubbleTea List demo running in browser, compiled by TinyGo!

image

Summary

Make the stubbed os/signal.signal_recv block forever instead of returning
immediately, so signal.Notify no longer starves cooperative schedulers on
wasm and baremetal targets.

Fixes #5619

Problem

src/runtime/signalstub.go (build tags tinygo.wasm || baremetal) stubbed
signal_recv as return ^uint32(0). Upstream's os/signal.loop calls
signal_recv in a tight loop with no yield point — it relies on the
runtime blocking until a signal arrives, as the POSIX implementation in
runtime_unix.go does. With the immediate-return stub, the watcher
goroutine started by signal.Notify spins forever.

On a cooperative scheduler a spinning goroutine is always runnable, so the
scheduler never idles: on wasm, _start never returns to the host event
loop and the browser tab (or node/wazero) pins a core with all other
goroutines starved. Any program calling signal.Notify is affected —
Bubble Tea does so unconditionally, which is how this surfaced (a V8
profile of the hung program showed 99.7% of ticks in os/signal.loop).

Fix

signal_recv now calls deadlock() — the same primitive a blocking empty
select uses — parking the watcher goroutine forever. That matches the
real implementation's observable behavior on a system where no signal ever
arrives: Notify succeeds, the channel simply never receives anything, and
everything else keeps running. This is also what gc's js/wasm port does.

Verification

  • New behavioral test testdata/signalnotify.go (signal.Notify, then
    time.Sleep, then print done), registered for all platforms. The
    existing signal.go test is skipped on wasm/baremetal/windows, which is
    why this had no coverage.
  • Red: on stock dev @ 86d58db the wasm test hangs until the Go test
    timeout kills it.
  • Green with this change: TestBuild/WebAssembly/signalnotify.go and
    TestBuild/Host/signalnotify.go both pass (the host run exercises the
    real POSIX signal_recv path, guarding both implementations).
  • End to end: a go-booba-adapted Bubble Tea bubbles/list app compiled for
    GOOS=js GOARCH=wasm previously froze the browser tab at 100% CPU right
    after startup; with this change it runs interactively.
  • deadlock() is defined by all four scheduler implementations
    (cooperative, threads, cores, none), covering the stub's whole build-tag
    surface.
  • Not run locally: cortex-m-qemu / AVR / RISC-V builds of the new test
    (this environment lacks the LLVM source checkout for compiler-rt); CI
    covers those. If os/signal turns out not to fit AVR flash, the test can
    be excluded for AVR the way json.go/stdlib.go are.

Context

Found while verifying the large-parameter-spilling work (#5615) in a real
browser; it is independent of that change and reproduces on stock dev.

The signalstub implementation of os/signal.signal_recv returned
^uint32(0) immediately. Upstream's os/signal.loop calls signal_recv in
a tight loop with no yield point, so the watcher goroutine started by
signal.Notify spun forever. On the cooperative wasm scheduler that
starves every other goroutine and never returns control to the host:
any program calling signal.Notify — Bubble Tea does, for example —
freezes the browser tab or wasm runner at 100% CPU (a V8 profile
showed 99.7% of ticks in os/signal.loop).

Signals can never arrive on these targets, so block forever using
deadlock(), the same primitive a blocking empty select uses. This
matches the real implementation's behavior while no signal is pending.

Add a behavioral test that runs on the stubbed platforms (the existing
signal.go test is skipped there): before this change it times out on
wasm; with it the sleep completes and the program exits.

Signed-off-by: Evan Wies <evan@neomantra.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os/signal: signal.Notify permanently starves the scheduler on wasm (stubbed signal_recv returns immediately)

1 participant